include/plateau/polygon_mesh/node.h (1)

57-58: 引数はconst参照で受け取りたい／未発見時の仕様をコメントで明記を

現状はstd::stringを値渡ししており、不要なコピーが発生します。const std::string&（必要ならstd::string_view）にすると軽量になります。また「見つからない場合は-1を返す」契約をヘッダに明記しておくと利用側に親切です。戻り値の先頭constは値型には効果がないため、スタイル上の冗長さが気になります（既存の宣言スタイルに合わせるならこのままでも可）。

以下のようにしてはどうでしょうか（宣言のみ差分）:

-        const int getChildIndexByName(const std::string name) const;
+        const int getChildIndexByName(const std::string& name) const; // 見つからない場合は -1 を返す

include/plateau/polygon_mesh/model.h (1)

43-43: 引数をconst参照化し、未発見時の戻り値についての契約を明記してください

値渡しは不要なコピーを生むため、const std::string&（もしくはstd::string_view）を推奨します。-1を返す仕様もコメントで明確化を。

-        const int getRootNodeIndexByName(const std::string name) const;
+        const int getRootNodeIndexByName(const std::string& name) const; // 見つからない場合は -1 を返す

src/polygon_mesh/model.cpp (1)

37-46: 署名のconst参照化と符号/サイズ不一致の軽減（size_t使用）

• 文字列は値渡しではなくconst参照に。
• ループ変数や要素数はsize_tを用いると不必要な符号変換を避けられます。返却は-1のためintのままでOKですが、返す際に明示castしておくと読みやすいです。

-    const int Model::getRootNodeIndexByName(const std::string name) const {
-        int num_children = getRootNodeCount();
-        for (int i = 0; i < num_children; ++i) {
-            auto& child_node = getRootNodeAt(i);
+    const int Model::getRootNodeIndexByName(const std::string& name) const {
+        size_t num_children = getRootNodeCount();
+        for (size_t i = 0; i < num_children; ++i) {
+            const auto& child_node = getRootNodeAt(i);
             if (child_node.getName() == name) {
-                return i;
+                return static_cast<int>(i);
             }
         }
         return -1;
     }

include/plateau/polygon_mesh/mesh_extract_options.h (1)

122-127: ドキュメントの明確化（「最高LOD」の定義を具体化）

「最高LOD」がデータセット内で検出された最大LODなのか、min_lod..max_lodの範囲に基づくmax_lodなのかが読み手に伝わりにくいです。利用側の期待値ズレを防ぐため、仕様をコメントで明記することを推奨します。

例（コメントのみの変更案）:

-        /**
-         * 最高LODのみを抽出するかどうかを指定します。
-         * true の場合、最高LODのポリゴンのみを抽出し、他のLODは無視します。
-         * false の場合、min_lod から max_lod までの範囲のポリゴンを抽出します。
-         */
+        /**
+         * 最高LODのみを抽出するかどうかを指定します。
+         * true の場合、min_lod..max_lod の範囲のうち最大のLOD（= max_lod）に相当するポリゴンのみを抽出し、
+         * 他のLODは無視します。false の場合、min_lod から max_lod までの範囲のポリゴンを抽出します。
+         * （もし「データセット内で検出された実在の最大LOD」のみを対象にする仕様であれば、その旨に書き換えてください）
+         */

include/plateau/polygon_mesh/mesh_extractor.h (1)

65-65: 型修飾の一貫性（geometry::Extent → plateau::geometry::Extent）を揃えましょう

同ヘッダ内の他API（Line 40, 45）は plateau::geometry::Extent を明示していますが、ここだけ geometry::Extent になっています。名前探索に依存せず一貫させた方が可読性と保守性が上がります。

適用例:

-        static std::vector<geometry::Extent> extendExtents(const std::vector<geometry::Extent>& src_extents, float multiplier);
+        static std::vector<plateau::geometry::Extent> extendExtents(const std::vector<plateau::geometry::Extent>& src_extents, float multiplier);

補足:

• 既に本ヘッダで他所でも std::vector を使用しているため必須ではありませんが、ヘッダ自身で <vector> をインクルードしておくと依存関係が明確になります。

src/polygon_mesh/node.cpp (1)

124-134: 署名とループの型の見直し（参照渡し/size_t/不要なconst）

• 文字列はコピーせず参照渡しに。
• getChildCount() は size_t を返すため、ループも size_t を使用。
• 返り値 const int の const は無意味なので削除。
• getChildAt の引数型（unsigned int）に合わせてキャスト。

-    const int Node::getChildIndexByName(const std::string name) const {
-        int num_children = getChildCount();
-        for (int i = 0; i < num_children; ++i) {
-            auto& child_node = getChildAt(i);
+    int Node::getChildIndexByName(const std::string& name) const {
+        const size_t num_children = getChildCount();
+        for (size_t i = 0; i < num_children; ++i) {
+            const auto& child_node = getChildAt(static_cast<unsigned int>(i));
             if (child_node.getName() == name) {
-                return i;
+                return static_cast<int>(i);
             }
         }
         return -1;
     }

ヘッダ側の宣言も合わせてください（別ファイルのため参考コード）:

// include/plateau/polygon_mesh/node.h
// 変更前: const int getChildIndexByName(const std::string name) const;
int getChildIndexByName(const std::string& name) const;

src/polygon_mesh/mesh_extractor.cpp (2)

62-64: ノード名に grid_id も含めて重複回避を検討してください

キーが (group_id, grid_id) に拡張された一方、ノード名は group{group_id} のみです。異なる grid_id で同一 group_id が存在すると、ノード名が重複して名前ベースの検索（本PRで追加された name→index 取得系）で曖昧になります。

重複回避の例:

-                for (auto& [group_grid_id, mesh] : result) {
-                    auto node = Node("group" + std::to_string(group_grid_id.first), std::move(mesh));
+                for (auto& [group_grid_id, mesh] : result) {
+                    const auto& [group_id, grid_id] = group_grid_id;
+                    auto node = Node("group" + std::to_string(group_id) + "_grid" + std::to_string(grid_id), std::move(mesh));
                     lod_node.addChildNode(std::move(node));
                 }

もしくは、Model/Node 側で重名ノードの許容・非許容ポリシーを明示しておくと後続のAPI利用者が誤解しにくいです。

---

219-232: 微最適化（reserve）と負値ガードの検討

処理は正しいですが、微小改善の余地があります。

• result.reserve(src_extents.size()) で再割り当てを抑制
• multiplier <= 0 の入力ガード（想定外値なら 1.0f に丸める等）

-    std::vector<geometry::Extent> MeshExtractor::extendExtents(const std::vector<geometry::Extent>& src_extents, float multiplier) {
-        auto result = std::vector<geometry::Extent>();
+    std::vector<geometry::Extent> MeshExtractor::extendExtents(const std::vector<geometry::Extent>& src_extents, float multiplier) {
+        auto result = std::vector<geometry::Extent>();
+        result.reserve(src_extents.size());
+        // 任意: multiplier の最低値ガード
+        // if (multiplier <= 0.f) multiplier = 1.f;

src/polygon_mesh/area_mesh_factory.h (2)

5-5: 不要なインクルードを削除してください

mesh_extractor.h から tile_extractor.h への変更は CityModelVector 型の使用に伴うものですが、TileExtractor クラス自体は使用されていないため、前方宣言で十分です。

-#include <plateau/polygon_mesh/tile_extractor.h>
+namespace plateau::polygonMesh {
+    using CityModelVector = std::shared_ptr<std::vector<std::weak_ptr<const citygml::CityModel>>>;
+}

---

11-11: 型名を明確にしてください

GridMergeResult の新しい定義では、キーが std::pair<unsigned, unsigned> になりましたが、この pair が何を表すのか（group_id, grid_id なのか、x, y座標なのか）が不明確です。型エイリアスまたはコメントで意味を明確にすることを推奨します。

-    /// グループIDと、その結合後Meshのmapです。
-    using GridMergeResult = std::map<std::pair<unsigned, unsigned>, std::unique_ptr<Mesh>>;
+    /// グループIDとグリッドIDのペアをキーとし、その結合後Meshのmapです。
+    /// キー: std::pair<group_id, grid_id>
+    using GridMergeResult = std::map<std::pair<unsigned, unsigned>, std::unique_ptr<Mesh>>;

src/polygon_mesh/area_mesh_factory.cpp (2)

121-127: 条件分岐のインデントが不適切です

highest_lod_only オプションのチェック部分のインデントがタブとスペースで混在しているように見えます。

-				if (options.highest_lod_only) {
-					// highest_lod_only オプションが有効な場合、最大LODのみを対象とします。
-					if (lod != max_lod_in_obj) {
-						// 最大LOD以外はスキップします。
-						continue;
-					}
-				}
+                if (options.highest_lod_only) {
+                    // highest_lod_only オプションが有効な場合、最大LODのみを対象とします。
+                    if (lod != max_lod_in_obj) {
+                        // 最大LOD以外はスキップします。
+                        continue;
+                    }
+                }

---

225-225: 固定キー (0,0) の使用意図を明確にしてください

combine メソッドで結果を固定キー (0,0) で格納していますが、この設計意図が不明確です。複数の CityModel を結合する際に単一のメッシュとして扱うためと推測されますが、コメントで明示することを推奨します。

 mesh_factory.optimizeMesh();
+// 複数のCityModelを結合した単一のメッシュとして、固定キー(0,0)で格納
 merged_meshes.emplace(std::make_pair(0,0), mesh_factory.releaseMesh());

src/polygon_mesh/tile_extractor.cpp (6)

1-10: ヘッダーファイルのインクルード順序を整理してください

インクルードの順序が一貫していません。通常は以下の順序が推奨されます：

1. 対応するヘッダー（tile_extractor.h）
2. 標準ライブラリ
3. 外部ライブラリ
4. プロジェクト内の他のヘッダー

また、area_mesh_factory.hのインクルードパスが相対パスになっています。

-#include "plateau/polygon_mesh/tile_extractor.h"
-#include <plateau/polygon_mesh/primary_city_object_types.h>
-#include "citygml/texture.h"
-#include "citygml/cityobject.h"
-#include "plateau/polygon_mesh/map_attacher.h"
-#include <plateau/polygon_mesh/mesh_factory.h>
-#include <plateau/polygon_mesh/polygon_mesh_utils.h>
-#include <plateau/dataset/gml_file.h>
-#include <plateau/texture/texture_packer.h>
-#include "area_mesh_factory.h"
+#include <plateau/polygon_mesh/tile_extractor.h>
+
+#include <plateau/polygon_mesh/area_mesh_factory.h>
+#include <plateau/polygon_mesh/map_attacher.h>
+#include <plateau/polygon_mesh/mesh_factory.h>
+#include <plateau/polygon_mesh/polygon_mesh_utils.h>
+#include <plateau/polygon_mesh/primary_city_object_types.h>
+#include <plateau/dataset/gml_file.h>
+#include <plateau/texture/texture_packer.h>
+#include <citygml/cityobject.h>
+#include <citygml/texture.h>

---

86-86: Line 86に不要な空白行があります

コードの一貫性のため、不要な空白行を削除してください。

 for (unsigned lod = options.min_lod; lod <= options.max_lod; lod++) {
-                       
     // 3D都市モデルをグリッドに分け、グリッドごとにメッシュをマージします。

---

99-122: グリッドノードの階層構造構築ロジックが複雑です

このコードブロックは深いネストと複数の条件分岐により可読性が低下しています。ヘルパー関数に分割することを検討してください。

以下のようなヘルパー関数の追加を提案します：

// 匿名名前空間内に追加
Node& findOrCreateGridNode(Model& model, const std::string& grid_name) {
    const auto grid_index = model.getRootNodeIndexByName(grid_name);
    if (grid_index == -1) {
        auto grid_node = Node(grid_name);
        return model.addNode(std::move(grid_node));
    }
    return model.getRootNodeAt(grid_index);
}

Node& findOrCreateLodNode(Node& parent_node, const std::string& lod_name) {
    const auto lod_index = parent_node.getChildIndexByName(lod_name);
    if (lod_index == -1) {
        auto lod_node = Node(lod_name);
        return parent_node.addChildNode(std::move(lod_node));
    }
    return parent_node.getChildAt(lod_index);
}

その後、Line 99-122を以下のように簡略化：

-const auto grid_index = out_model.getRootNodeIndexByName(grid_name);
-if (grid_index == -1) {
-    // 存在しない場合 新しいグリッドノードとLODノードを作成
-    auto grid_node = Node(grid_name);
-    auto& new_grid_node = out_model.addNode(std::move(grid_node));			
-    auto lod_node = Node(lod_name);
-    lod_node.addChildNode(std::move(node));
-    new_grid_node.addChildNode(std::move(lod_node));
-}
-else {
-    auto& grid_node = out_model.getRootNodeAt(grid_index);  
-    const auto lod_index = grid_node.getChildIndexByName(lod_name);
-    if (lod_index == -1) {
-        // LODノードが存在しない場合、新しいLODノードを作成
-        auto lod_node = Node(lod_name);
-        lod_node.addChildNode(std::move(node));
-        grid_node.addChildNode(std::move(lod_node));
-    }
-    else {
-        // 既存のLODノードにノードを追加
-        grid_node.getChildAt(lod_index).addChildNode(std::move(node));
-    }
-}
+auto& grid_node = findOrCreateGridNode(out_model, grid_name);
+auto& lod_node = findOrCreateLodNode(grid_node, lod_name);
+lod_node.addChildNode(std::move(node));

---

29-30: エクステントの拡張係数がハードコードされています

1.2fという拡張係数がハードコードされています。この値を定数として定義するか、オプションパラメータとして設定可能にすることを検討してください。

+namespace {
+    constexpr float EXTENT_EXPANSION_FACTOR = 1.2f;
+}

 // Line 30付近
-auto extents = MeshExtractor::extendExtents(extents_before_adjust, 1.2f);
+auto extents = MeshExtractor::extendExtents(extents_before_adjust, EXTENT_EXPANSION_FACTOR);

 // Line 83付近
-auto extents = MeshExtractor::extendExtents(extents_before_adjust, 1.2f);
+auto extents = MeshExtractor::extendExtents(extents_before_adjust, EXTENT_EXPANSION_FACTOR);

Also applies to: 82-83

---

124-134: else節のブレース配置が不一致です

Line 124のelseがLine 125の開き中括弧と別の行になっています。プロジェクトのコーディング規約に従った一貫したスタイルを使用してください。

-else 
-{
+else {

---

187-193: highest_lod_only オプションの分岐意図をコメントで明示してください

tile_extractor.cpp の 187–193 行で、

• options.highest_lod_only == false の場合は MeshExtractor::extractInExtents を呼び出し、
• true の場合は extractWithGridInner を呼び出していますが、
  その理由がソース上からは読み取りにくくなっています。

さらに、area_mesh_factory.cpp でも同一オプションで LOD をフィルタリングしている実装があるため、
なぜグリッドベースの抽出（extractWithGridInner）時にのみこの分岐を行う必要があるのかを、
以下いずれかの箇所にコメントで補足してください。

• 関数定義部（extractTilesInExtents の冒頭付近）
• オプション構造体定義部（MeshExtractOptions のフィールド説明）

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/c_wrapper/tile_extractor_c.cpp (2)

4-4: 未使用インクルードの削除を検討

この翻訳単位では geo_coordinate.h は参照していません。ビルド時間・可読性の観点で削除可です。

適用例:

-#include <plateau/geometry/geo_coordinate.h>

---

19-27: 全要素が nullptr の場合の戻り値ポリシーを要確認

全てスキップされ city_models->empty() になるケースで、現在は内部処理に進みます。API としては ErrorInvalidArgument か ErrorValueNotFound を返す方が呼び出し側にとって明確です。ポリシーに応じて上記の早期リターンをご採用ください。必要なら全呼び出し箇所の期待値確認を支援します。

src/polygon_mesh/tile_extractor.cpp (5)

33-34: LOD ループ変数の型は int の方が安全

options.min_lod が符号付きで負値入りうる場合、unsigned への暗黙変換で想定外の巨大ループになり得ます。int にしましょう。

適用例:

-        for (unsigned lod = options.min_lod; lod <= options.max_lod; lod++) {
+        for (int lod = options.min_lod; lod <= options.max_lod; ++lod) {

---

85-86: 同上: Grid 側の LOD ループも int へ

一貫性と安全性のため、こちらも int に。

適用例:

-        for (unsigned lod = options.min_lod; lod <= options.max_lod; lod++) {
+        for (int lod = options.min_lod; lod <= options.max_lod; ++lod) {

---

95-97: 一時オブジェクトへの参照束縛を避ける（微修正）

const auto& は一時 std::string に束縛されています。可読性のため値で持つと明確です。

適用例:

-                    const auto& grid_name = "GRID" + std::to_string(group_grid_id.second + 1); // 1,2,3,4
-                    const auto& lod_name = "LOD" + std::to_string(lod);
+                    const std::string grid_name = "GRID" + std::to_string(group_grid_id.second + 1); // 1,2,3,4
+                    const std::string lod_name = "LOD" + std::to_string(lod);

---

18-19: の明示的インクルードを追加

std::filesystem を使用しているため、自前で <filesystem> をインクルードしておくと移植性が上がります（間接インクルードに依存しない）。

適用例:

 #include <plateau/texture/texture_packer.h>
 #include "area_mesh_factory.h"
+#include <filesystem>

---

99-121: 名前検索の都度線形探索になるため、将来的なスケールでの性能が気になる（任意）

getRootNodeIndexByName / getChildIndexByName を毎回探す実装は O(N) です。グリッドと LOD の骨格を先に構築してインデックスをキャッシュするか、ハッシュマップで索引を保持する設計も検討余地があります。

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/TileExtractor.cs:71-75
Timestamp: 2025-08-18T07:04:09.540Z
Learning: sevendev prefers to use int for cityModelCount in C# P/Invoke signatures and is willing to modify the C++ side to use int instead of size_t for city_model_size parameter in tile_extractor_c.cpp functions.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

src/c_wrapper/tile_extractor_c.cpp (1)

29-37: shared_ptr の参照返しを避け、追加の null 監視を推奨

getCityModelPtr() の戻りが値返しの場合でも参照束縛で期間延長は成立しますが、可読性と誤解防止のため値で受ける方が安全です。また、shared_ptr 自体が空のケースをスキップする軽微なガードを入れておくと堅牢性が上がります。ついでに capacity を予約して小さな最適化を。

適用例:

-            CityModelVector city_models = std::make_shared<std::vector<std::weak_ptr<const citygml::CityModel>>>();
+            CityModelVector city_models = std::make_shared<std::vector<std::weak_ptr<const citygml::CityModel>>>();
+            city_models->reserve(static_cast<size_t>(city_model_size));
             for (int i = 0; i < city_model_size; ++i) {
                 if (!city_model_handles[i]) {
                     continue; // nullptr の場合はスキップ  
                 }
-                const auto & ptr = city_model_handles[i]->getCityModelPtr(); // ここで city_model_handles[i] のポインタを取得
+                auto ptr = city_model_handles[i]->getCityModelPtr(); // 共有所有権を値で受ける
+                if (!ptr) {
+                    continue; // 空の shared_ptr はスキップ
+                }
                 std::weak_ptr<const citygml::CityModel> weak = ptr;
                 city_models->push_back(weak);
             }

src/polygon_mesh/tile_extractor.cpp (3)

33-51: combine 呼び出し前に city_models の null/空チェックを追加すると安全

現状、combine は city_models をデリファレンスするため、万一 nullptr が渡される経路（C++ 直接呼び出し等）があると未定義動作になります。C ラッパー経由ではすでに防げていますが、内部関数として早期 return を入れておくと防御的で安全です。

適用例:

     // rootNode として LODノード を作ります。
     for (unsigned lod = options.min_lod; lod <= options.max_lod; lod++) {
         auto lod_node = Node("LOD" + std::to_string(lod));

         // LODノードの下にメッシュ配置用ノードを作ります。
         {
+            if (!city_models || city_models->empty()) {
+                // 入力なしの場合はこの LOD をスキップ
+                continue;
+            }
             // 次のような階層構造を作ります:

---

91-123: 一時オブジェクトへの参照束縛は動作上は安全だが、明示的な文字列へ置換すると読みやすい

const auto& で一時 std::string に寿命延長をかけるのは正しいですが、可読性の観点では実体を持つ std::string 変数にするのが無難です。

適用例:

-                    const auto& grid_name = "GRID" + std::to_string(group_grid_id.second + 1); // 1,2,3,4
-                    const auto& lod_name = "LOD" + std::to_string(lod);
+                    const std::string grid_name = "GRID" + std::to_string(group_grid_id.second + 1); // 1,2,3,4
+                    const std::string lod_name = "LOD" + std::to_string(lod);

---

147-154: grid 経路でも GmlFile 生成前にパスの空判定を入れると堅牢

combine では空判定を入れましたが、grid では未実施です。念のため空文字（あるいは存在しないパス）時はスキップする条件を合わせると一貫性が出ます。

適用例:

-        // 現在の都市モデルが地形であるなら、衛星写真または地図用のUVを付与し、地図タイルをダウンロードします。
-        auto package = GmlFile(city_model.getGmlPath()).getPackage();
-        if (package == PredefinedCityModelPackage::Relief && options.attach_map_tile) {
+        // 現在の都市モデルが地形であるなら、衛星写真または地図用のUVを付与し、地図タイルをダウンロードします。
+        const auto& gmlPath = city_model.getGmlPath();
+        if (!gmlPath.empty()) {
+            auto package = GmlFile(gmlPath).getPackage();
+            if (package == PredefinedCityModelPackage::Relief && options.attach_map_tile) {
                 const auto gml_path = fs::u8path(city_model.getGmlPath());
                 const auto map_download_dest = gml_path.parent_path() / (gml_path.filename().u8string() + "_map");
                 MapAttacher().attach(out_model, options.map_tile_url, map_download_dest, options.map_tile_zoom_level,
-                geo_reference);
-        }
+                    geo_reference);
+            }
+        }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/TileExtractor.cs:71-75
Timestamp: 2025-08-18T07:04:09.540Z
Learning: sevendev prefers to use int for cityModelCount in C# P/Invoke signatures and is willing to modify the C++ side to use int instead of size_t for city_model_size parameter in tile_extractor_c.cpp functions.

src/c_wrapper/tile_extractor_c.cpp (2)

18-26: 外部入力のバリデーション追加が適切（combine API）

nullptr/負値/不整合入力（size>0 かつ配列null）を早期に ErrorInvalidArgument で弾いており、過去指摘のクラッシュ経路が解消されています。

---

28-40: nullptr と空 shared_ptr のスキップで安全性が確保されています

city_model_handles[i] の nullptr と getCityModelPtr() の空をいずれもスキップする実装は妥当です。reserve 済みなので余計な再割当も避けられています。

src/c_wrapper/tile_extractor_c.cpp (2)

55-66: grid() 側も CityModel の空チェックを入れるとより堅牢です

combine 側では getCityModelPtr() を用いて空をスキップ/検出していますが、grid 側は参照を直接取得しているため、ハンドルが空の可能性を考慮できません。以下のように getCityModelPtr() を使って検証してから参照渡しするとクラッシュ耐性が上がります。

-            TileExtractor::extractWithGrid(*out_model, city_model_handle->getCityModel(), options, *extents);
+            auto cityModelPtr = city_model_handle->getCityModelPtr();
+            if (!cityModelPtr) {
+                return APIResult::ErrorInvalidArgument;
+            }
+            TileExtractor::extractWithGrid(*out_model, *cityModelPtr, options, *extents);

---

11-11: const 修飾の精緻化（配列の二重ポインタ）を検討してください

読み取り専用 API 意図が明確になるよう、配列要素ポインタまで const にするとより堅牢です（ABI 影響は新規 API の今なら小）。

-        const CityModelHandle** const city_model_handles,
+        const CityModelHandle* const* city_model_handles,

変更する場合は対応するヘッダーと P/Invoke 宣言の更新も忘れずに。

src/polygon_mesh/mesh_extractor.cpp (2)

46-46: 範囲拡張のメソッド化は妥当。係数1.2fは定数化（またはオプション化）を検討してください。

固定値のマジックナンバーは読みやすさと将来の調整容易性を下げます。例えば MeshExtractOptions に拡張倍率フィールドを追加する、もしくはファイル内で constexpr として定義すると意図が明確になります。

必要であれば、Options 追加または定数化のパッチを用意します。要望ください。

---

220-233: extendExtents の仕様を明確化（高さ方向含有の明記）と引数バリデーションの追加を提案。

実装は全軸（幅・奥行き・高さ）を中心対称にスケールしていますが、コメントは「幅と奥行き」と読める表現です。ドキュメントと実装の乖離を避けるため文言調整を推奨します。併せて multiplier のバリデーションを入れると公開APIとして堅牢になります。

コメントの明確化（行220）と軽いバリデーション追加の例:

-    /// extentsの幅と奥行きの長さを multiplier 倍にします。
+    /// extents の各軸（幅・奥行き・高さ）を中心点基準で multiplier 倍に拡張／縮小します。
     std::vector<plateau::geometry::Extent> MeshExtractor::extendExtents(const std::vector<plateau::geometry::Extent>& src_extents, float multiplier) {
         auto result = std::vector<plateau::geometry::Extent>();
         result.reserve(src_extents.size());
+        // multiplier の簡易バリデーション：0以下は不正、1.0 は無変換を早期返却
+        if (multiplier <= 0.0f) {
+            throw std::invalid_argument("MeshExtractor::extendExtents: multiplier must be > 0.");
+        }
+        if (multiplier == 1.0f) {
+            return src_extents;
+        }
         for (const auto& src_extent : src_extents) {
             const auto center = src_extent.centerPoint();
             const auto prev_min = src_extent.min;
             const auto prev_max = src_extent.max;
             auto next_min = center + (prev_min - center) * multiplier;
             auto next_max = center + (prev_max - center) * multiplier;
             result.emplace_back(next_min, next_max);
         }
         return result;
     }

補足: XY のみを拡張すべき要件であれば、その旨の仕様化とコード修正（Z成分のみ multiplier=1 固定）をご検討ください。

XY のみ拡張版へのパッチも提示可能です。必要であれば方向性（全軸 or XY限定）をご指定ください。

src/polygon_mesh/area_mesh_factory.h (1)

30-35: combine の公開に伴う契約の明文化を推奨（キー順序と安定性）。

戻り値の GridMergeResult は pair<group_id, grid_id> の昇順で列挙されます。呼び出し側が順序依存（例: 先頭を「全体」扱い等）するなら契約としてコメント化しておくと誤用を減らせます。

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/TileExtractor.cs:71-75
Timestamp: 2025-08-18T07:04:09.540Z
Learning: sevendev prefers to use int for cityModelCount in C# P/Invoke signatures and is willing to modify the C++ side to use int instead of size_t for city_model_size parameter in tile_extractor_c.cpp functions.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

include/plateau/polygon_mesh/polygon_mesh_types.h (1)

3-7: 前方宣言と不要インクルード削減でビルド時間・依存を軽減しましょう

このヘッダ内では CityModelVector のみを定義しており、citygml::CityModel は weak_ptr のテンプレート実引数として参照するだけです。citymodel.h の重いインクルードは不要で、citygml::CityModel の前方宣言で済みます。また <map> と <utility> は未使用です。自己完結性を保ちつつ、依存を最小化する方向を提案します。

適用例:

-#include "citygml/citymodel.h"
-#include <map>
-#include <memory>
-#include <utility>
-#include <vector>
+#include <memory>
+#include <vector>
+
+namespace citygml {
+    class CityModel;
+}

src/polygon_mesh/area_mesh_factory.h (2)

3-7: 標準ライブラリの必須ヘッダを明示してヘッダの自己完結性を担保してください

本ヘッダは宣言段階で std::map / std::pair / std::unique_ptr を使用しますが、<map> / <memory> / <utility> を直接インクルードしていません。トランジティブ依存に頼らず、ここで明示しておくべきです。

追加例:

 #include "citygml/citymodel.h"
 #include "plateau/geometry/geo_reference.h"
 #include <plateau/polygon_mesh/mesh.h>
 #include <plateau/polygon_mesh/mesh_extract_options.h>
 #include <plateau/polygon_mesh/polygon_mesh_types.h>
+#include <map>
+#include <memory>
+#include <utility>

---

32-34: CityModelVector は const 参照で受け取ってコピーを避けましょう

共有ポインタの浅いコピーとはいえ不要な参照カウント変更を避け、意図（読み取り専用）を明確にできます。TileExtractor も同様に合わせると一貫します。

-        combine(CityModelVector city_models, const MeshExtractOptions& options, unsigned lod,
+        combine(const CityModelVector& city_models, const MeshExtractOptions& options, unsigned lod,
             const plateau::geometry::GeoReference& geo_reference, const std::vector<plateau::geometry::Extent>& extents);

include/plateau/polygon_mesh/tile_extractor.h (2)

24-26: CityModelVector を const 参照で受け取りに変更を

API の意図（不変）を明確にし、参照カウント操作を減らせます。呼び出し元互換性も維持されます。

-        static std::shared_ptr<Model> extractWithCombine(
-            CityModelVector city_models, const MeshExtractOptions& options,
+        static std::shared_ptr<Model> extractWithCombine(
+            const CityModelVector& city_models, const MeshExtractOptions& options,
             const std::vector<plateau::geometry::Extent>& extents);
@@
-        static void extractWithCombine(Model& out_model,
-            CityModelVector city_models, const MeshExtractOptions& options,
+        static void extractWithCombine(Model& out_model,
+            const CityModelVector& city_models, const MeshExtractOptions& options,
             const std::vector<plateau::geometry::Extent>& extents);

Also applies to: 31-33

---

6-8: 未使用インクルードの削減でビルドを軽量化

本ヘッダの宣言には mesh.h と geo_coordinate.h は不要に見えます（Model は model.h、CityModel は citygml/citymodel.h で賄えています）。削ることで依存とビルド時間を抑えられます。

-#include <plateau/polygon_mesh/mesh.h>
-#include <plateau/geometry/geo_coordinate.h>

src/c_wrapper/tile_extractor_c.cpp (1)

1-4: std::vector を明示インクルードして自己完結性を高める

本ファイルはシグネチャで std::vector<plateau::geometry::Extent> を参照します。トランジティブ依存に頼らず <vector> を明示した方が安全です。

 #include "libplateau_c.h"
 #include "city_model_c.h"
 #include <plateau/polygon_mesh/tile_extractor.h>
+#include <vector>

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/TileExtractor.cs:71-75
Timestamp: 2025-08-18T07:04:09.540Z
Learning: sevendev prefers to use int for cityModelCount in C# P/Invoke signatures and is willing to modify the C++ side to use int instead of size_t for city_model_size parameter in tile_extractor_c.cpp functions.

src/c_wrapper/tile_extractor_c.cpp (1)

55-69: grid 抽出の二重呼び出し問題は解消されています

前回指摘の重複呼び出しがなくなり、ヌル検証→単一呼び出しの直線的な制御に整理されています。

src/polygon_mesh/area_mesh_factory.h (2)

7-10: ヘッダ自己完結性の不足: を明示インクルードしてください

本ヘッダの関数シグネチャで std::vector を用いていますが、直接の <vector> インクルードがありません（polygon_mesh_types.h 経由のトランジティブに依存）。将来の変更で壊れやすいため、自己完結させましょう。

 #include <plateau/polygon_mesh/polygon_mesh_types.h>
 #include <map>
 #include <memory>
 #include <utility>
+#include <vector>

---

35-37: 引数の受け方（shared_ptr）は方針次第。現状の const& でもOK

CityModelVector は小さな shared_ptr なので値渡しでもオーバーヘッドは僅少です。APIとして「所有権共有」を明示したい（内部でコピーして保持する等）場合は値渡し、完全に読み取り専用で参照だけなら const& のままで問題ありません。現状のままでも十分です。

src/c_wrapper/tile_extractor_c.cpp (5)

1-1: UTF-8 BOM を削除してください（ビルド環境によっては警告/不具合の原因）

先頭に BOM（）が混入しています。UTF-8 (no BOM) へ整形してください。

---

1-4: を明示インクルードしてください

本ファイルの関数シグネチャで std::vector<...> を直接使用しています。トランジティブに依存せず、明示的に <vector> を追加してください。

 #include "libplateau_c.h"
 #include "city_model_c.h"
 #include <plateau/polygon_mesh/tile_extractor.h>
+#include <vector>

---

5-7: using namespace は削除を推奨（局所化）

実装ファイルとはいえ、C APIラッパのトップレベルに using namespace は不要です。該当箇所は限定的なので、完全修飾名に切り替えて可読性と衝突リスクを下げましょう。

-using namespace libplateau;
-using namespace plateau::polygonMesh;
+// using namespace は避け、必要箇所で完全修飾にします。

あわせて、下記の宣言を完全修飾に:

-            CityModelVector city_models = std::make_shared<std::vector<std::weak_ptr<const citygml::CityModel>>>();
+            plateau::polygonMesh::CityModelVector city_models =
+                std::make_shared<std::vector<std::weak_ptr<const citygml::CityModel>>>();

---

10-15: extern "C" に C++ STL 型が現れています（C ABI 観点での注意喚起）

extern "C" で公開する関数に std::vector（のポインタ）が含まれるため、厳密な C ABI 互換は満たしません。もし本APIが真のC境界越え（他言語/他コンパイラ）で使用されるなら、POD配列+サイズ等に置き換える検討を推奨します。現状どおり同一ツールチェーン内でしか使わない設計なら問題にはなりません。

必要であれば、Extent* extents, size_t extents_count 形のCフレンドリーな薄いラッパ関数の追加パッチを用意します。

---

28-40: CityModelVector の構築ロジックは良好です

• 事前 reserve による再確保回数の低減
• ヌルハンドル/空 shared_ptr のスキップ
• weak_ptr への変換で生存管理を呼び出し元に委譲

いずれも妥当です。細かい改善としては emplace_back(ptr); への置換で一段階コピーを省けますが、パフォーマンス影響は極小です。

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/TileExtractor.cs:71-75
Timestamp: 2025-08-18T07:04:09.540Z
Learning: sevendev prefers to use int for cityModelCount in C# P/Invoke signatures and is willing to modify the C++ side to use int instead of size_t for city_model_size parameter in tile_extractor_c.cpp functions.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

src/polygon_mesh/area_mesh_factory.h (2)

14-16: 2Dキーに別名を与えて可読性を向上しませんか？

キー型が std::pair<unsigned, unsigned> だと読み手が「どちらが group でどちらが grid か」を都度コメントに頼ることになります。薄いラッパー alias を挟むだけでも意図が明瞭になります。

適用例:

-    // グループIDとグリッドIDのペアをキーとし、その結合後Meshのmapです。
-    // キー: std::pair<group_id, grid_id>
-    using GridMergeResult = std::map<std::pair<unsigned, unsigned>, std::unique_ptr<Mesh>>;
+    // グループIDとグリッドIDのペアをキーとし、その結合後Meshのmapです。
+    // キー: GroupGridKey (＝ std::pair<group_id, grid_id>)
+    using GroupGridKey = std::pair<unsigned, unsigned>;
+    using GridMergeResult = std::map<GroupGridKey, std::unique_ptr<Mesh>>;

---

33-38: combine の役割とキー仕様をコメントで明記 + [[nodiscard]] の付与を提案

APIの意図（グループ×グリッド単位で返す／キー=(group_id, grid_id)）をヘッダ側コメントに残すと利用者が混乱しません。返り値を無視すべきでない関数なので [[nodiscard]] も付けておくと堅いです。

-        /**
-         * 複数のcity_model内のメッシュを結合して返します。
-         */
-        static GridMergeResult
+        /**
+         * 複数の city_model を対象に、グリッド分割→グループ分けを行い、
+         * グループ×グリッド単位で結合したメッシュを返します。
+         * 戻り値 GridMergeResult のキーは (group_id, grid_id) です。
+         * options.highest_lod_only を尊重します。
+         */
+        [[nodiscard]] static GridMergeResult
         combine(const CityModelVector& city_models, const MeshExtractOptions& options, unsigned lod,
             const plateau::geometry::GeoReference& geo_reference, const std::vector<plateau::geometry::Extent>& extents);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/TileExtractor.cs:71-75
Timestamp: 2025-08-18T07:04:09.540Z
Learning: sevendev prefers to use int for cityModelCount in C# P/Invoke signatures and is willing to modify the C++ side to use int instead of size_t for city_model_size parameter in tile_extractor_c.cpp functions.

src/polygon_mesh/mesh_extractor.cpp (2)

46-46: 倍率 1.2f のハードコードは定数化 or オプション化を検討

境界スキップ率や誤差吸収のチューニング値なので、埋め込み値よりも定数名 or 設定経由にした方が保守しやすいです。少なくとも識別子を与えて意味を明示してください。

以下のように置き換えを提案します（Line [46]）:

-        auto extents = MeshExtractor::extendExtents(extents_before_adjust, 1.2f);
+        auto extents = MeshExtractor::extendExtents(extents_before_adjust, kExtentExpandMultiplier);

追加で、定数宣言をこのファイル内（例えば無名名前空間の先頭付近）に置いてください:

// 無名名前空間の先頭など
static constexpr float kExtentExpandMultiplier = 1.2f;

---

219-232: multiplier の異常値に対する安全策がない（<= 0 で退化/反転の恐れ）

multiplier <= 0 だと退化（0）や min/max 反転（負値）を引き起こす可能性があります。API を公開したので防御的に早期リターン or clamp/アサートの追加を推奨します。

簡易対策（関数冒頭）:

-    std::vector<plateau::geometry::Extent> MeshExtractor::extendExtents(const std::vector<plateau::geometry::Extent>& src_extents, float multiplier) {
+    std::vector<plateau::geometry::Extent> MeshExtractor::extendExtents(const std::vector<plateau::geometry::Extent>& src_extents, float multiplier) {
+        if (multiplier <= 0.0f) {
+            // 退化・反転を避ける
+            return src_extents;
+        }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/MeshExtractOptions.cs (3)

86-92: コンストラクタ内の二重代入を削除し、検証ロジックに一本化しましょう

min/max LOD、UnitScale、GridCountOfSide、MapTileURL は直後に検証付きのメソッド/プロパティで再設定されており、現状は二重代入です。フィールド直書きを削除して、検証経路のみを通す方が安全で読みやすいです。

適用例（該当箇所削除）:

-            this.minLOD = minLOD;
-            this.maxLOD = maxLOD;
-            this.unitScale = unitScale;
-            this.gridCountOfSide = gridCountOfSide;
-            this.mapTileURL = mapTileURL;

---

216-227: MapTileURL セッターに null と長さ制約の検証追加を提案

ByValTStr(SizeConst=1000) は固定長で、過長は切り詰められます。明示的に検証しておくと不意のトリミングを防げます。StartsWith の比較も大文字小文字無視を推奨します。

提案パッチ:

-                if (!value.StartsWith("http"))
+                if (string.IsNullOrWhiteSpace(value))
+                {
+                    throw new ArgumentException("URL must not be null or empty.");
+                }
+                if (!value.StartsWith("http", StringComparison.OrdinalIgnoreCase))
                 {
                     throw new ArgumentException("URL must start with http.");
                 }
+                // ByValTStr(SizeConst=1000): 実使用可能な最大文字数は 1000 未満を想定
+                if (value.Length >= 1000)
+                {
+                    throw new ArgumentException("URL is too long. Limit is less than 1000 characters.");
+                }

---

229-229: EpsgCode へのリネームが破壊的変更になります
外部コードで旧フィールド epsgCode を直接参照しているとビルドエラーになります。

• v4 で破壊的変更を許容する場合：リリースノートで明記してください
• 互換性を維持する場合：暫定的にエイリアスプロパティを提供することを検討してください

影響箇所

• wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/MeshExtractOptions.cs の public int EpsgCode;

互換性維持の例

[Obsolete("Use EpsgCode instead.")]
public int epsgCode { get => EpsgCode; set => EpsgCode = value; }

参照箇所の確認コマンド

rg -nP --type=cs -C2 '\bepsgCode\b'

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.064Z
Learning: `area_mesh_factory.cpp`の`gridMerge`では、`group_id`は`grid_id * 100 + max_lod_in_obj`として計算されるため、同一の`group_id`を持つ異なる`grid_id`のペアは存在しない。したがって、`mesh_extractor.cpp`では`group_grid_id.first`のみでノード名を構築しても重複は発生しない。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.064Z
Learning: `AreaMeshFactory::gridMerge`では、`group_id = grid_id * (max_lod_in_specification_ + 1) + max_lod_in_obj`の計算式により、各`group_id`は`grid_id`と`max_lod`の組み合わせから一意に決まる。そのため、`mesh_extractor.cpp`の文脈では`group_id`だけでノード名を構築しても重複は発生しない。

data/日本語パステスト/codelists/TransportationDataQualityAttribute_srcScale.xml (1)

1-3: BOM の有無をフォルダ内で統一してください

本ファイルは UTF-8 BOM 付きですが、同ディレクトリの RoadStructureAttribute_sectionType.xml は BOM なしです。ツールチェーンやプラットフォームによっては差分ノイズや取り込み不整合の原因になります。UTF-8 は BOM 不要が一般的のため、全ファイルで「BOM なし」に統一することを推奨します。

data/日本語パステスト/codelists/RoadStructureAttribute_sectionType.xml (2)

1-52: BOM なしで保存されています（他ファイルと不一致）

本ファイルは BOM なし、他の追加ファイルは BOM ありが混在しています。フォルダ全体で UTF-8（BOM なし）へ統一することを推奨します。

---

35-51: gml:id 命名規則の不一致とコード値の飛び（8 が欠番）があります

• 先頭6件は gml:id が RoadStructureAttribute_sectionType_1〜6 ですが、後半2件のみ id7/id8 と命名が異なります。
• gml:name のコードが 1,2,3,4,5,6,7,9 と並び、8 が欠番です。仕様通りであればコメントや説明の追記、あるいは定義の追加/修正が必要かもしれません。

命名規則は揃えた方が保守性が上がります。仕様が許すなら以下のようなリネームを検討してください（コード値の変更は仕様確認後に判断）。

-<gml:Definition gml:id="id7">
+<gml:Definition gml:id="RoadStructureAttribute_sectionType_7">
   <gml:description>橋・高架</gml:description>
   <gml:name>7</gml:name>
 </gml:Definition>
 ...
-<gml:Definition gml:id="id8">
+<gml:Definition gml:id="RoadStructureAttribute_sectionType_8">
   <gml:description>不明</gml:description>
   <gml:name>9</gml:name>
 </gml:Definition>

注: 上記は gml:id のみを命名規則に揃える案です。gml:name の「9」が仕様上正しいのか（8 を意図的に使用しないのか）を確認してください。

data/日本語パステスト/codelists/TransportationDataQualityAttribute_thematicSrcDesc.xml (1)

1-4: データファイルの配布・参照経路をビルドに組み込んでください

これら codelist XML は実行時参照が前提と思われます。以下をご確認ください:

• CMake/ビルドスクリプトでインストール対象に含めているか（例: install(DIRECTORY data/... DESTINATION share/...））。
• 実行時の検索パスに当該ディレクトリが含まれるか。
• 日本語を含むパス（日本語パステスト）を Windows などで正しく扱えるか（ワイド文字 API/UTF-8 対応）。

必要であれば CMake の例を提示します。

例（CMake）:

# 例: プロジェクトのデータとしてインストール
install(
  DIRECTORY data/日本語パステスト/codelists/
  DESTINATION ${CMAKE_INSTALL_DATADIR}/libplateau/codelists
  FILES_MATCHING PATTERN "*.xml"
)

test/test_tile_extractor.cpp (10)

4-7: 未使用のインクルードを削除してコンパイル時間と依存を減らす

grid_code.h と gml_file.h は本テスト内で参照がありません。削除可能です。

-#include "plateau/dataset/grid_code.h"
-#include <plateau/dataset/gml_file.h>

---

38-40: CTAD（型推論）依存を避けて明示的に型を指定

ビルド設定やコンパイラによっては std::vector の型推論が無効/不明瞭になる可能性があります。可読性のためにも明示を推奨します。

-        std::vector extents = {
+        std::vector<Extent> extents = {
             Extent(GeoCoordinate(min.x, min.y, min.z), GeoCoordinate(max.x, max.y, max.z))
         };

---

44-44: アサーションがテストデータ依存で脆い（最大4のはず、というコメントと不一致）

コメントでは「最大4」とありますが、ASSERT_EQ(..., 4) 固定だと対象データの中身に依存して落ちやすいです。範囲チェックに緩めると安定します。

-        ASSERT_EQ(model->getRootNodeCount(), 4); // 2x2のグリッドなので最大4つのGRIDノードがあるはず（オブジェクトがなければ少なくなる）
+        const auto root_count = model->getRootNodeCount();
+        ASSERT_GE(root_count, 1);  // 少なくとも1つはあるはず
+        ASSERT_LE(root_count, 4);  // 2x2 グリッドの最大

---

47-55: 子ノードアクセス前に存在チェックを追加

getChildAt(0) の前に getChildCount() を確認しておくと、将来の仕様変更やデータ差異での落ち方がわかりやすくなります。

         const auto& grid_node = model->getRootNodeAt(0);
         const auto& grid_name = grid_node.getName();
         ASSERT_EQ(grid_name.substr(0, 4), "GRID");
 
         // LODノードの名前を確認
-        const auto& lod_node = grid_node.getChildAt(0);
+        ASSERT_GT(grid_node.getChildCount(), 0);
+        const auto& lod_node = grid_node.getChildAt(0);
         const auto& lod_name = lod_node.getName();
         ASSERT_EQ(lod_name.substr(0, 3), "LOD");

---

56-60: 更に一段下の子ノードも存在チェックしてから参照

同様に一段下の getChildAt(0) も件数チェックを先に。

         // グリッドノードの子ノードの名前を確認
-        const auto& first_model_node = lod_node.getChildAt(0);
+        ASSERT_GT(lod_node.getChildCount(), 0);
+        const auto& first_model_node = lod_node.getChildAt(0);
         const auto& first_model_node_name = first_model_node.getName();
         ASSERT_EQ(first_model_node_name.substr(0, 5), "group");

---

25-26: GML ロード結果のヌルチェックを追加して原因切り分けを容易に

パスやエンコーディング問題があると load が失敗する可能性があります。アサートを足しておくとデータ/環境不備が一目でわかります。

-        const std::shared_ptr<const CityModel> city_model = load(gml_path, params_);
+        const std::shared_ptr<const CityModel> city_model = load(gml_path, params_);
+        ASSERT_TRUE(city_model);

-        const std::shared_ptr<const CityModel> city_model1 = load(gml_path1, params_);
-        const std::shared_ptr<const CityModel> city_model2 = load(gml_path2, params_);
+        const std::shared_ptr<const CityModel> city_model1 = load(gml_path1, params_);
+        const std::shared_ptr<const CityModel> city_model2 = load(gml_path2, params_);
+        ASSERT_TRUE(city_model1);
+        ASSERT_TRUE(city_model2);

Also applies to: 74-76

---

82-85: CTAD（型推論）依存を避けて明示的に型を指定（combine 側）

grid テストと同様に、明示型指定を推奨します。

-        std::vector extents = {
+        std::vector<Extent> extents = {
             Extent(GeoCoordinate(min1.x, min1.y, min1.z), GeoCoordinate(max1.x, max1.y, max1.z)),
             Extent(GeoCoordinate(min2.x, min2.y, min2.z), GeoCoordinate(max2.x, max2.y, max2.z)),
         };

---

94-99: ルートノード存在チェックを先に

念のためルートの存在を先にアサートしてから 0 番目を参照。

-        // LODノードの名前を確認
+        // LODノードの名前を確認
+        ASSERT_GT(model->getRootNodeCount(), 0);
         const auto& lod_node = model->getRootNodeAt(0);
         const auto& lod_name = lod_node.getName();
         ASSERT_EQ(lod_name.substr(0, 3), "LOD");

---

47-50: （任意）全 GRID ルートノード名を一括検証

0番目だけでなく、全ルートノードが "GRID" で始まることを一括で確認すると、回 regressions に強くなります。

例（挿入用イメージ、実際の位置はお好みで）:

for (size_t i = 0; i < model->getRootNodeCount(); ++i) {
    const auto& node = model->getRootNodeAt(i);
    const auto& name = node.getName();
    ASSERT_EQ(name.substr(0, 4), "GRID");
}

---

24-24: テストデータの存在チェックを自動化し、相対パス依存を低減
相対パス(../data/日本語パステスト/...)は実行ディレクトリの違いでテストが失敗するリスクがあるため、以下の対応を検討してください。

• std::filesystem を使い、テスト実行前にファイルの存在を明示的にチェック
  例:

  namespace fs = std::filesystem;
  const fs::path data_root = TEST_DATA_DIR;  // CMake で -DTEST_DATA_DIR="…/data" を定義
  const auto gml_path = (data_root / u8"日本語パステスト" / "udx" / "tile_test" / "52353549_tran_6697_op.gml").u8string();
  REQUIRE(fs::exists(gml_path));  // 存在しなければテスト失敗

• CMake 側で TEST_DATA_DIR のような変数を定義し、絶対パスを組み立てることで “実行ディレクトリ依存” を排除

対象箇所

• test/test_tile_extractor.cpp: 24行目の const std::string gml_path = …
• 同ファイル 72–74行目 にも同様の相対パス参照あり

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.064Z
Learning: `area_mesh_factory.cpp`の`gridMerge`では、`group_id`は`grid_id * 100 + max_lod_in_obj`として計算されるため、同一の`group_id`を持つ異なる`grid_id`のペアは存在しない。したがって、`mesh_extractor.cpp`では`group_grid_id.first`のみでノード名を構築しても重複は発生しない。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.064Z
Learning: `AreaMeshFactory::gridMerge`では、`group_id = grid_id * (max_lod_in_specification_ + 1) + max_lod_in_obj`の計算式により、各`group_id`は`grid_id`と`max_lod`の組み合わせから一意に決まる。そのため、`mesh_extractor.cpp`の文脈では`group_id`だけでノード名を構築しても重複は発生しない。

wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/MeshExtractOptions.cs (1)

231-232: HighestLodOnly に [MarshalAs(UnmanagedType.U1)] が付与され、マーシャリングずれの懸念は解消しています

過去コメントで指摘されていたABIずれは解消されています。対応ありがとうございます。

wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/MeshExtractOptions.cs (2)

156-160: UnitScale のバリデーションを value 基準に修正した点は妥当。例外型は ArgumentOutOfRangeException を推奨

機能的にはOKです。例外は範囲不正なので ArgumentOutOfRangeException のほうが文脈に合います。

以下の最小差分を提案します:

-                {
-                    throw new ArgumentException($"Validate failed : {nameof(this.UnitScale)} is too small.");
-                }
+                {
+                    throw new ArgumentOutOfRangeException(nameof(value), "UnitScale is too small.");
+                }

---

231-232: HighestLodOnly に C++ 側と同等のXMLドキュメントを付与することを推奨

利用者向けに意味が明確になります（C++ヘッダのコメントを反映）。

-        [MarshalAs(UnmanagedType.U1)]
-        public bool HighestLodOnly;
+        /// <summary>
+        /// 最高LODのみを抽出するかどうかを指定します。
+        /// true の場合、minLOD..maxLOD のうち最大LODに相当するポリゴンのみを抽出します。
+        /// false の場合、minLOD から maxLOD までの範囲のポリゴンを抽出します。
+        /// </summary>
+        [MarshalAs(UnmanagedType.U1)]
+        public bool HighestLodOnly;

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.064Z
Learning: `AreaMeshFactory::gridMerge`では、`group_id = grid_id * (max_lod_in_specification_ + 1) + max_lod_in_obj`の計算式により、各`group_id`は`grid_id`と`max_lod`の組み合わせから一意に決まる。そのため、`mesh_extractor.cpp`の文脈では`group_id`だけでノード名を構築しても重複は発生しない。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.064Z
Learning: `area_mesh_factory.cpp`の`gridMerge`では、`group_id`は`grid_id * 100 + max_lod_in_obj`として計算されるため、同一の`group_id`を持つ異なる`grid_id`のペアは存在しない。したがって、`mesh_extractor.cpp`では`group_grid_id.first`のみでノード名を構築しても重複は発生しない。

Learnt from: sevendev
PR: Synesthesias/libplateau#281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.782Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。